Add support of ddl and dml sql statements to SqlToSubstrait#428
Add support of ddl and dml sql statements to SqlToSubstrait#428ZorinAnton wants to merge 1 commit intosubstrait-io:mainfrom
Conversation
fe86d02 to
9ce46e3
Compare
create table as and create view to SqlToSubstrait9ce46e3 to
3561c70
Compare
3561c70 to
b9a1c61
Compare
vbarua
left a comment
There was a problem hiding this comment.
This PR motivated me to continue some ongoing work for #362, specifically #430.
Effectively, the plan is to break SqlToSubstrait down into a conversion of SQL into Calcite, and then Calcite into Substrait. The DdlRelBuilder you've introduced goes against this work because it couples the SQL to the Substrait translation. It also seems somewhat duplicative, because I think we should be able to use the Calcite machinary to convert DDL SqlNodes into RelNodes, and then all we would need would be your SubstraitRelVisitor updates.
| var parsedList = parser.parseStmtList(); | ||
| SqlToRelConverter converter = createSqlToRelConverter(validator, catalogReader); | ||
| // IMPORTANT: parsedList gets filtered in the call below | ||
| List<io.substrait.plan.Plan.Root> ddlRelRoots = ddlSqlToRootNodes(parsedList, converter); |
There was a problem hiding this comment.
Do we need to process DDL statements separately? Would it be possible to just parse all statements, and then just use the conversion code in SubstraitRelVisitor?
|
I think it's a good idea to break down SqlToSubstrait into Sql->Calcite and Calcite->Substrait, as it will give better flexibility. |
Ah, TIL. Poking around your PR, the DML statements I want to think more about the DDL API, and honestly kind of want to land it after #430. That being said, would you be open to splitting the DML and DDL processing into separate PRs? The DML works is pretty straightforward and should be easier to review on it's own. |
DDL:
For the above mentioned sql statements create the following relations:
NamedWriteforcreate table asNamedDdlforcreate viewAll other types of sql statements are handled as before.
DML:
The class
SubstraitRelVisitornow supports the following operations for theTableModifynode: